Dispatcher Build Gate Summary (local)

• rubocop (diff-only): PASS (0 offenses)
• brakeman: skipped (per repo build gate config; disabled)
• tests (affected engines): PASS
  • core: PASS
  • admin: PASS (testing/ change triggers full engine run)
  • storefront: PASS

Note: test output still prints BSON Symbol deprecation warning (expected to be addressed by WA-NEW-010 / PR #635).

Security Review

Verdict: PASS_WITH_NOTES (LOW)

No direct authn/z, injection, or secret-handling impact observed. Main concern is guarding destructive test helpers.

Notes

• testing/lib/workarea/test_case.rb — truncate_all_mongoid_clients! iterates all configured Mongoid clients and deletes all non-system collections. Great for test isolation, but broadly destructive if tests are ever pointed at a non-test/shared DB.

  • Recommendation (P1): add an explicit safety guard (e.g., raise unless Rails.env.test?) and optionally assert DB name looks like a test DB.

• *core/app/workers/workarea/ ** — workers now rescue Mongoid::Errors::DocumentNotFound and no-op. Reasonable for callback workers after truncation, but it reduces visibility into unexpected deletes/mis-enqueues.

  • Recommendation (P3): consider debug logging / instrumentation when rescuing to preserve diagnostic signal.

Simplicity Review

Verdict: PASS_WITH_NOTES

Changes are pragmatic and easy to follow.

Notes

• core/app/workers/workarea/bust_navigation_cache.rb:17 — Several workers repeat the same rescue Mongoid::Errors::DocumentNotFound; nil pattern.

  • Suggestion: if this spreads further, consider a tiny shared helper/module wrapper to centralize the behavior; otherwise duplication is fine for now.

• testing/lib/workarea/test_case.rb:71 — wait_for_elasticsearch! uses an inline retry loop with a long multi-exception rescue.

  • Suggestion: if it grows, consider Timeout.timeout(timeout) around a small retry block or extract the rescued exception list into a constant. Current version is still contained/readable.

Architecture Review

Verdict: PASS_WITH_NOTES (LOW)

Overall: coherent change — callback workers become idempotent/no-op when backing Mongoid docs are gone; test support gains more complete reset helpers (ES readiness + truncating all Mongoid clients).

Notes

• Workers (multiple files) — Many workers now inline rescue Mongoid::Errors::DocumentNotFound; nil. This matches the goal, but it’s duplicated and only one worker includes rationale comments.

  • Suggestion: centralize via a small concern/base helper (e.g., noop_on_missing_document! / with_document(id) { ... }) and/or standardize a short comment across workers.

• testing/lib/workarea/test_case.rb — New helpers are in the right layer (test support), but embedded inside specific concerns, which could limit discoverability if reused elsewhere.

  • Suggestion: if these helpers spread, consider extracting to a dedicated helper module namespace; otherwise current placement is fine.

• core/app/workers/workarea/synchronize_user_metrics.rb — Only this worker has an explicit rationale comment for swallowing missing-doc errors.

  • Suggestion: add the same short rationale (or reference the issue) across all touched workers, or centralize the behavior with documentation.

Rails Conventions Review

Verdict: PASS_WITH_NOTES (LOW)

Changes generally follow Rails conventions (jobs are more tolerant/idempotent). A few places could be made more idiomatic/consistent.

Notes

• core/app/workers/workarea/bust_navigation_cache.rb:17 — Inline rescue DocumentNotFound is repeated across workers and can become noisy/rescue-as-control-flow.

  • Suggestion: centralize missing-doc→no-op behavior in a shared helper/base worker, or use a non-raising lookup and return early.

• core/app/workers/workarea/generate_promo_codes.rb:9 — rescue ...; nil reads a bit odd and intent isn’t consistently documented.

  • Suggestion: prefer rescue Mongoid::Errors::DocumentNotFound plus a short comment / return for clarity.

• testing/lib/workarea/test_case.rb:71 — wait_for_elasticsearch! retry loop works but could be made more idiomatic.

  • Suggestion: extract the check into a small helper and use a clearer begin/rescue/retry (or Timeout.timeout(timeout) around the loop).

✅ Review summary

Wave 1 reviewers are all PASS or PASS_WITH_NOTES.

• Architecture: PASS_WITH_NOTES (LOW)
• Simplicity: PASS_WITH_NOTES (LOW)
• Security: PASS_WITH_NOTES (LOW)
• Rails conventions: PASS_WITH_NOTES (LOW)

Labeled merge:ready.